Skip to content

Conversation

@Xyedo
Copy link

@Xyedo Xyedo commented May 6, 2024

fixing this issues: #63
and appending the testcase on #64:

By letting golang time package calculate the daylight savings by changing it to time.Duration and add it to the existing Date

@leaanthony-sc
Copy link

leaanthony-sc commented May 20, 2024

I've discovered an edge case where this does not work: If you have a DAILY frequency and it crosses a timeline, then it will fail.

Test added to rrule_test.go:

func TestDailyDST(t *testing.T) {
	sydney, _ := time.LoadLocation("Australia/Sydney")
	r, _ := NewRRule(ROption{
		Freq:    DAILY,
		Count:   3,
		Dtstart: time.Date(2023, 4, 1, 9, 0, 0, 0, sydney),
	})
	want := []time.Time{
		time.Date(2023, 4, 1, 9, 0, 0, 0, sydney),
		time.Date(2023, 4, 2, 9, 0, 0, 0, sydney),
		time.Date(2023, 4, 3, 9, 0, 0, 0, sydney),
	}
	value := r.All()
	if !timesEqual(value, want) {
		t.Errorf("get %v, want %v", value, want)
	}
}

Running it gives us the following error:

=== RUN   TestDailyDST
    rrule_test.go:1751: get [2023-04-01 09:00:00 +1100 AEDT 2023-04-02 08:00:00 +1000 AEST 2023-04-03 09:00:00 +1000 AEST], want [2023-04-01 09:00:00 +1100 AEDT 2023-04-02 09:00:00 +1000 AEST 2023-04-03 09:00:00 +1000 AEST]
--- FAIL: TestDailyDST (0.00s)

I believe for DAILY we need to recreate the date but only add the number of days.

Xyedo and others added 2 commits May 20, 2024 21:28
This reverts commit c969eda.

chore: revert back to original solution and add fixes
@Xyedo
Copy link
Author

Xyedo commented May 20, 2024

I think for FREQ hourly, minutely, and secondly we need to add with time duration, so the duration is fixed, but with FREQ Daily, Weekly, and Monthly, we follow the time provided by the DTSTART and then add the Date.

@leaanthony-sc
Copy link

This change appears to work. Awesome job @Xyedo! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants